Finish photometry notebooks #381
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #381 +/- ##
===========================================
- Coverage 78.49% 52.12% -26.38%
===========================================
Files 27 56 +29
Lines 3758 5815 +2057
===========================================
+ Hits 2950 3031 +81
- Misses 808 2784 +1976 ☔ View full report in Codecov by Sentry. |
mwcraig
force-pushed
the
finsih-photometry-notebooks
branch
from
5ada920 to
8bdd8c6
Compare
ignore-words.txt
Outdated
| @@ -2,3 +2,4 @@ TOI | |||
| toi | |||
| LIVETIME | |||
| lightyear | |||
| fo | |||
There was a problem hiding this comment.
You want to be able to use two-letter variable names? :)
mwcraig
force-pushed
the
finsih-photometry-notebooks
branch
3 times, most recently
from
8daeb9a to
96ac58e
Compare
Co-authored-by: Juan Cabanela <[email protected]>
Just like #JuanCab said I would need to in his review, which I swear I did actually read
mwcraig
force-pushed
the
finsih-photometry-notebooks
branch
from
6607656 to
65f4e5b
Compare
|
@JuanCab -- this is ready for a look. Sorry it got a little long. Once this is merged it will be possible to perform photometry via notebooks. I'll work on a separate PR to add some more documentation and another to make the command line approach is fairly straightforward. I'm not sure why coverage thinks we need to test our test files.... |
There was a problem hiding this comment.
I only reviewed commit 41416b3 (WIP photometry run settings) forward... and that is enough (24 commits)!
Most of the comments have to deal with the fact that Codecov was really freaking out demanding tests of tests and missing code that was getting tested. A few of the comments are more question/informational. No showstoppers and the tests look good.
| from .photometry_widget_functions import * | ||
| from .profile_and_comps import * |
There was a problem hiding this comment.
As near as I can tell this warning about code coverage is in error! You test these functions with gui_tools/tests/test_profile_and_comps.py. Not sure why it is failing. Possibly because you import ComparisonAndSeeing instead of profile_and_comps (yes, they are equivalent, but I don't know if Codecov knows that).
There was a problem hiding this comment.
Wait, Codecov is complaining that the tests are not covered by tests? Is it trying for "Inception"?
The new test itself looks fine.
There was a problem hiding this comment.
Again, looks like Codecov is trying out for "Inception 2: The Test Never Stop" by asking for tests of test. Weird behavior. Furthermore, this tests the code that Codecov earlier claimed was not getting tested (line 5 of stellarphot/gui_tools/__init__.py).
|
|
||
| class TestComparisonAndSeeing: | ||
| def copy_fits_to_temp(self, tmp_path): | ||
| # Copy a fits file to a temporary directory |
There was a problem hiding this comment.
Is this just testing you can write the FITS file to the temporary directory? I assume as much, but goal of test wasn't clear from comments.
There was a problem hiding this comment.
This isn't a test, it just takes care of copying the FITS file needed for a couple of the other tests in the class
This pull requests finishes off the notebooks needed to perform photometry in 2.0.
Still to do:
NOTE: